Skip to content

fix: address review findings across recent PR churn (governance caveat, skeleton hang, owner sentinel, port probe)#321

Merged
BigSimmo merged 5 commits into
mainfrom
claude/pr-review-issues-2ewx3t
Jul 6, 2026
Merged

fix: address review findings across recent PR churn (governance caveat, skeleton hang, owner sentinel, port probe)#321
BigSimmo merged 5 commits into
mainfrom
claude/pr-review-issues-2ewx3t

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes from a full review of the ~30 recently merged PRs (search/answer UI churn, mobile fixes, Supabase security/drift hardening, dependency bumps):

  • Governance (follow-up to feat: promote all indexed documents public regardless of validation status #294): 20260706120000 promoted every indexed document into the public anonymous corpus regardless of clinical_validation_status, and new uploads default to unverified — but the unverified_source warning was filtered out of the frontend-visible set, so answers grounded on never-locally-validated guidelines showed no caveat at all. unverified_source is now frontend-visible (still non-blocking; review_due_source stays a hidden routine note). Tests updated to pin the new intent.
  • Answer skeleton hang (residual case of Fix answer search URL chat loading #316): the !canRunSearch early-return in executeSearch fires after searchRequestSeqRef is bumped but never cleared loading, and the bump gates the superseded in-flight request's finally out of clearing it either — so the answer skeleton could stay on screen indefinitely. It now resets loading/answerProgress like its sibling early-returns.
  • DB (follow-up to fix(db): reconcile live database drift and harden security without RAG regression #300/fix(rag): deploy app-layer public owner sentinel for anonymous search #291): match_document_embedding_fields_text (codified in 20260705230000) kept the legacy (owner_filter is null or d.owner_id = owner_filter) predicate and ignores the public-owner sentinel: the anonymous sentinel would match zero rows and a real owner id would exclude public documents. New migration 20260706130000 recreates it with retrieval_owner_matches (with the fix(db): repair Supabase Preview migration replay failures #305-style replay guard); schema.sql updated to match. Latent today (only the _hybrid variant is wired in). Prepared, NOT applied to live — tracked in docs/process-hardening.md.
  • Local server scripts: canListenOnHost treated any listen error as "port busy", so in IPv6-less containers EAFNOSUPPORT on ::1/:: disqualified every port and both npm run dev and the Playwright runner died with "No free Playwright server port found". An unsupported address family now no longer disqualifies a port. Also added an opt-in PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH for sandboxes that block browser downloads (default behavior unchanged).
  • Deps: bumped the lingering eslint-config-next 16.2.9 pin to 16.2.10, aligning with next (chore(deps): bump next from 16.2.9 to 16.2.10 #311).

Verification

  • npm run verify:cheap — pass (check:runtime, sitemap:check, lint, typecheck, 1097 vitest tests)
  • npm run verify:ui — pass (103/103 Chromium Playwright, run with the preinstalled sandbox browser via the new env var)
  • npm run verify:release — not run; no release/handoff claim
  • npm run format:check — pass
  • npm run eval:retrieval:quality — not applicable: no retrieval ranking/selection/chunking/scoring behavior changed (the migration fixes an owner predicate on an RPC that app code does not call; needs live keys regardless)
  • npm run eval:rag / eval:quality — not applicable: answer generation and post-processing unchanged (only warning visibility in the UI)
  • npm run check:production-readiness — run; fails only on missing secrets in this demo-mode sandbox (NEXT_PUBLIC_SUPABASE_URL, SUPABASE_SERVICE_ROLE_KEY, OPENAI_API_KEY), expected per AGENTS.md
  • npm run check:deployment-readiness — not applicable: no deployment startup/hosting/rollout change

Clinical Governance Preflight

  • Source-backed claims still require linked source verification before clinical use — refusal behavior unchanged; this PR only adds a visible caveat
  • No patient-identifiable document workflow was introduced or expanded
  • Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy)
  • Service-role keys and private document access remain server-only — the fixed RPC keeps service_role-only execute
  • Demo/synthetic content remains clearly separated from real clinical sources
  • Source metadata, review status, and outdated/unknown-source behavior remain conservative — strictly more conservative: the unvalidated-source caveat is now shown instead of hidden
  • Deployment classification/TGA SaMD impact checked — answer content and refusal logic unchanged; only an additional warning is displayed

Notes

  • Migration 20260706130000_fix_embedding_fields_text_owner_sentinel must be applied to live with the next approved push; until then the fix exists only in the repo (the RPC is latent — not called by app code).
  • Review findings that did not need code changes here are listed in the review summary: document_chunks.owner_id left stale by the feat: promote all indexed documents public regardless of validation status #294 promotion (harmless — retrieval and RLS key off the parent document), a duplicate create or replace block in the applied 20260705210000 migration (do not rewrite history), and drift in the universal-search-command-mockups route.

Generated by Claude Code


Note

Medium Risk
Touches clinically visible governance warnings and search loading state; the retrieval migration is prepared but not applied live and the RPC is unused by app code today.

Overview
Follow-up fixes from recent search/governance and Supabase drift work.

Clinical governance UI: With the public corpus promotion, answers can cite never-locally-validated guidelines without any visible caveat because unverified_source was filtered out of frontend-visible warnings. That code is now shown (still non-blocking); review_due_source stays hidden as routine metadata. Tests pin the new visibility rules.

Search UX: When executeSearch bails early because !canRunSearch after bumping the search request id, it now clears loading and answerProgress so the answer skeleton cannot stick indefinitely (superseded in-flight finally blocks no longer clear it).

Database (prepared, not live): match_document_embedding_fields_text is updated to use retrieval_owner_matches instead of the legacy owner predicate so anonymous/public-owner filtering matches other retrieval RPCs. Migration 20260706130000 plus schema.sql sync; documented as latent until the _text RPC is wired in app code.

Tooling: canListenOnHost in dev and Playwright runners no longer treats EAFNOSUPPORT / EADDRNOTAVAIL as “port busy” on IPv6-less containers. Playwright optionally honors PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH for sandboxes with a preinstalled Chromium. eslint-config-next bumps 16.2.9 → 16.2.10.

Reviewed by Cursor Bugbot for commit 6dbd5b4. Configure here.

claude added 5 commits July 6, 2026 09:40
Since 20260706120000 promoted every indexed document into the public
anonymous corpus regardless of clinical_validation_status, and new uploads
default to "unverified", answers can be grounded on never-locally-validated
guidelines. The unverified_source warning was generated server-side but
filtered out of the frontend-visible set, so no caveat ever reached the
answer UI. Make it frontend-visible (still non-blocking; review_due stays a
hidden routine note).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011WXsrao2p3qYRuf7P9pkEY
This early-return fires after searchRequestSeqRef is bumped, which gates a
superseded in-flight request's finally block out of resetting loading, so
the answer skeleton could stay on screen indefinitely — the residual case
of the #316 skeleton-hang class. Mirror the sibling early-returns.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011WXsrao2p3qYRuf7P9pkEY
…fields_text

The codification in 20260705230000 kept the legacy
(owner_filter is null or d.owner_id = owner_filter) predicate, so this RPC
ignores retrieval_owner_matches: the anonymous sentinel would match zero
rows and a real owner id would exclude public documents. Latent (only the
_hybrid variant is wired into the app) but wrong the moment it is used.
Migration is prepared, NOT applied to live — tracked in
docs/process-hardening.md.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011WXsrao2p3qYRuf7P9pkEY
…um executable override

canListenOnHost resolved false on any listen error, so in IPv6-less
containers EAFNOSUPPORT on ::1/:: disqualified every port and both npm run
dev and the Playwright runner died with 'No free port found'. Treat an
unsupported address family as unable to hold the port.

Also let PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH point the chromium project at
a preinstalled browser for sandboxes that block browser downloads; default
behavior is unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011WXsrao2p3qYRuf7P9pkEY
Aligns the lingering 16.2.9 pin with next 16.2.10 (bumped in #311).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011WXsrao2p3qYRuf7P9pkEY
@supabase

supabase Bot commented Jul 6, 2026

Copy link
Copy Markdown

Updates to Preview Branch (claude/pr-review-issues-2ewx3t) ↗︎

Deployments Status Updated
Database Mon, 06 Jul 2026 09:42:33 UTC
Services Mon, 06 Jul 2026 09:42:33 UTC
APIs Mon, 06 Jul 2026 09:42:33 UTC

Tasks are run on every commit but only new migration files are pushed.
Close and reopen this PR if you want to apply changes from existing seed or migration files.

Tasks Status Updated
Configurations Mon, 06 Jul 2026 09:42:40 UTC
Migrations Mon, 06 Jul 2026 09:43:27 UTC
Seeding Mon, 06 Jul 2026 09:43:31 UTC
Edge Functions Mon, 06 Jul 2026 09:43:32 UTC

View logs for this Workflow Run ↗︎.
Learn more about Supabase for Git ↗︎.

@BigSimmo
BigSimmo marked this pull request as ready for review July 6, 2026 12:37
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@BigSimmo
BigSimmo merged commit 2e66255 into main Jul 6, 2026
6 checks passed
@BigSimmo
BigSimmo deleted the claude/pr-review-issues-2ewx3t branch July 8, 2026 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants